Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@extensions/cornerstone/src/commandsModule.ts`:
- Around line 202-240: The function _getRegionSegmentPlusLimits uses direct
instanceof checks for StackViewport and VolumeViewport but those classes are not
imported and bypass the legacy GenericViewport adapters; replace the instanceof
checks by using the existing helper predicates isStackViewportType(viewport) and
isVolumeViewportType(viewport) (importing them if missing) to detect viewport
types, keep the same logic for computing maxDeltaK/maxDeltaIJ, and ensure
csUtils.getImageSliceDataForVolumeViewport(viewport) is only called when
isVolumeViewportType returns true; update imports to include the predicate
functions rather than StackViewport/VolumeViewport classes.

---

Outside diff comments:
In `@modes/segmentation/src/toolbarButtons.ts`:
- Around line 766-822: The toolbar button currently uses id 'RegionSegmentPlus'
but setToolActiveToolbar resolves targets from toolName || itemId || value so it
may activate the wrong legacy id; update the commands for this button (the
commands array that currently includes 'setToolActiveToolbar') to pass the
flood-fill tool name explicitly (e.g., include a command entry invoking
setToolActiveToolbar with toolName: 'RegionSegmentPlusFloodFill' or set the
value to 'RegionSegmentPlusFloodFill') so activation targets the registered
tool; locate the button definition (id: 'RegionSegmentPlus') in
modes/segmentation/src/toolbarButtons.ts and modify the commands element to
supply the explicit tool name when calling setToolActiveToolbar.

---

Nitpick comments:
In `@playwright.config.ts`:
- Around line 3-4: E2E_PORT is set using Number(...) without validation which
can produce NaN for bad env values; replicate the utility script's validation:
read process.env.OHIF_PORT, if present parse it as an integer (base 10), ensure
it's an integer between 1 and 65535, and if invalid throw a descriptive Error
stating the invalid OHIF_PORT value and expected range; if not present, use the
default 3335. Then set E2E_PORT and E2E_BASE_URL using the validated integer.
Reference the constants E2E_PORT and E2E_BASE_URL and mirror the validation
behavior from free-ohif-e2e-port.mjs.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/build-docs.yml:
- Line 81: The gh run download step uses the unvalidated expression `${{
steps.find_pr_run.outputs.run_id }}` which could be abused if its format
changes; fix by validating that `steps.find_pr_run.outputs.run_id` contains only
digits before invoking `gh run download` (e.g., add a prior step that checks
`/^\d+$/` and fails if not) and then use the validated value (or a sanitized
output variable) in the `gh run download` command (also wrap the injected value
in quotes or pass it via a safe flag) so the `run_id` is guaranteed numeric and
cannot be used for command injection.

In `@extensions/cornerstone-dicom-sr/package.json`:
- Around line 23-25: The dev script in modes/segmentation's package.json calls
rspack with .webpack/webpack.dev.js but that file is missing; add a
.webpack/webpack.dev.js for modes/segmentation (or change the dev script) so the
dev build works. Fix by creating modes/segmentation/.webpack/webpack.dev.js that
mirrors the pattern used in extensions/cornerstone-dicom-sr and
modes/usAnnotation: require or import the shared .webpack/webpack.base.js, set
mode to "development" and export the config (ensuring compatibility with
`@rspack/core`), or alternatively change the dev script to point to the existing
.webpack/webpack.prod.js if that is intentional.

In `@modes/tmtv/package.json`:
- Around line 31-38: The peerDependencies block in modes/tmtv/package.json (and
likewise in extensions/cornerstone/package.json) currently uses monorepo-only
"workspace:*" ranges for entries like "`@ohif/core`",
"`@ohif/extension-cornerstone`", "`@ohif/extension-default`", etc.; replace every
"workspace:*" with an appropriate semver range (e.g. ^<version> or a range
matching the published package versions) so the published package exposes real
semver peer ranges; ensure you update each dependency key in the
"peerDependencies" object to the chosen semver (matching the root/monorepo
published versions) and run npm pack / npm publish dry-run to verify no
workspace protocol remains.

In `@platform/cli/src/commands/utils/getYarnInfo.js`:
- Around line 4-5: getYarnInfo currently returns JSON.parse(stdout) directly
from execa('npm', ['info', packageName, '--json']) which can be either an object
or an array; normalize the parsed result to preserve the previous return shape
by detecting if the parsed value is an array and selecting the appropriate
element (e.g., first item or the one matching packageName) or converting an
array into a single object with expected fields (peerDependencies, version,
etc.), and ensure getYarnInfo always returns an object with those fields; update
getYarnInfo to handle null/undefined stdout and throw or return a consistent
empty shape, and add a small unit test that simulates npm info returning both an
object and an array for the same package spec to assert the normalized shape is
returned.

In `@platform/core/package.json`:
- Line 21: The package.json currently sets "sideEffects" as a string ("false")
which can be misinterpreted by bundlers; change the value to the boolean false
(i.e., sideEffects: false) in the package.json so bundlers correctly treat the
package as side-effect-free and enable proper tree-shaking, ensuring the JSON
remains valid.

In `@platform/docs/docs/migration-guide/3p12-to-3p13/node-version.md`:
- Line 59: Update the CI reference line to use the proper product name and
capitalization: replace the lowercase "GitHub" occurrence in the phrase "CI
config (`.github/workflows/*`, `.circleci/config.yml`, " with the correctly
capitalized product name "GitHub Actions" so the line reads something like "CI
config (GitHub Actions `.github/workflows/*`, `.circleci/config.yml`, ..."
ensuring consistent capitalization and explicit mention of "GitHub Actions".
- Line 25: The doc string in node-version.md lists the pnpm engine as "pnpm":
"11.1.1" but templates now declare "pnpm": ">=11" in dependencies.json; update
node-version.md to match the generated manifest (change the documented pnpm
requirement to ">=11" or otherwise mirror the exact engine string used in
platform/cli/templates/mode/dependencies.json) and check the other occurrences
noted (lines 56–58) for the same mismatch so the documentation and template stay
consistent.

In `@platform/docs/docs/migration-guide/3p12-to-3p13/package-manager.md`:
- Around line 156-162: The package.json script examples contain malformed pnpm
invocations: update the script values for "dev:my-extension" and "start" from
"pnpm rundev" to "pnpm run dev", and update "build:package" from "pnpm runbuild"
to "pnpm run build" so the "pnpm run <script>" form is used correctly (look for
the "dev:my-extension", "build:package", and "start" entries to locate and fix
the strings).

In `@platform/ui-next/package.json`:
- Around line 9-17: The package.json currently exposes subpath exports (e.g.,
"./tailwind.config", "./lib/*", "./components/*", ".") that point at source
files (tailwind.config.js and src/**) but the "files" array only includes "dist"
and "README.md", causing published consumers to miss those exported paths; fix
by updating the "exports" entries to reference built outputs under dist (for
example change "./tailwind.config" and the "./lib/*" and "./components/*"
mappings to point at the corresponding files in dist) or alternatively add
"tailwind.config.js" and the relevant "src" paths to the "files" array so those
exported subpaths are included in the package. Ensure changes touch the
package.json keys "exports" and/or "files" so exports and published content
align.

In `@platform/ui/package.json`:
- Line 13: Update the repo documentation to state Node 24 is the new supported
baseline: make sure the package.json engines.node entry (engines.node) and
.node-version align with Node 24, then add a clear note in the README and
release notes/changelog indicating "Node >=24 (Node 24 LTS)" as the minimum
supported version, call out breaking compatibility with Node 18/20/22, and add a
short migration snippet (recommended upgrade steps and an nvm/snippet
suggestion) so consumers know how to upgrade; also mention where CI/Docker were
updated (CI node-version and Docker FROM node:24-slim) so readers understand the
change is repo-wide.

In `@publish-version.mjs`:
- Around line 88-95: The push of the tag (the execa call that currently runs
execa('git', ['push', 'origin', tagName'])) can fail if the remote already has
that tag because you only force the local tag (git tag -f) but not the remote
push; update the tag push to either include --force (execa('git', ['push',
'--force', 'origin', tagName'])) to match the local behavior or wrap the
existing execa call in a try/catch and on failure delete the remote tag (git
push origin :refs/tags/<tagName> or git tag -d <tagName> remote deletion) then
retry pushing tagName; locate the occurrences of tagName and branchName and the
execa('git', ...) calls to implement the change.

In `@tests/pages/RightPanelPageObject.ts`:
- Around line 227-231: The `rows` locator and getSegmentCount use a global
page.getByTestId('data-row'), causing flaky counts; scope them to the
segmentation panel root instead. Define a segmentation-panel-scoped locator
(e.g., segmentationRoot = page.getByTestId('segmentation-panel') or the existing
segmentation panel locator) and replace occurrences so rows =
segmentationRoot.getByTestId('data-row'); update getSegmentCount and any helpers
that reference rows/index/text to reuse that scoped rows locator.

---

Outside diff comments:
In @.github/workflows/playwright.yml:
- Around line 113-114: The Windows port-cleanup is using netstat + findstr with
a substring match (in .scripts/ci/free-ohif-e2e-port.mjs), which can match ports
like 33350 when you meant 3335; update the win32 branch to match the exact port:
either replace the netstat|findstr pipeline with a PowerShell query
(Get-NetTCPConnection -LocalPort <port> | Where-Object State -eq 'Listen' to get
exact listeners and their PIDs) or change the findstr invocation to a strict
regex (e.g., findstr /R /C:":<port>\\b" or include the trailing space/column
delimiter) so only the exact port is matched before killing processes (adjust
logic in the function that builds the command in free-ohif-e2e-port.mjs).

In `@platform/app/package.json`:
- Line 4: Remove the duplicate JSON key by deleting the later occurrence of
"productVersion" so only a single "productVersion": "3.4.0" remains; locate the
duplicate key named "productVersion" (the second occurrence near the end of the
file) and remove that entry, then validate the package.json syntax (e.g., with
JSON lint or npm pack) to ensure no trailing commas or structural errors remain.

---

Nitpick comments:
In `@extensions/measurement-tracking/package.json`:
- Line 42: The package.json declares "webpack-merge" in both peerDependencies
and devDependencies which is redundant; remove the duplicate entry from
peerDependencies and keep "webpack-merge" only in devDependencies (or vice-versa
if you intentionally require consumers to provide it) and update the
peerDependencies block accordingly; also check and remove the duplicate
occurrences referenced around the other reported lines (50-52) so
"webpack-merge" appears only once in the manifest.
- Line 35: The package.json currently lists "`@ohif/ui`" in both dependencies and
peerDependencies which causes ambiguity; remove the duplicate by keeping it only
in peerDependencies (so the host app provides it) and delete the "`@ohif/ui`"
entry from dependencies (also remove any duplicate entries around lines 44-49
mentioned in the review) to ensure a single source of truth for resolution.

In `@platform/docs/docs/platform/modes/index.md`:
- Around line 454-459: The note in the "modes" documentation uses the ambiguous
phrase "generating modes with an agent" without specifying which agent or
linking to relevant docs; update the note in the index.md "modes" section to
clarify what "agent" means (e.g., specify "AI agent", "automation agent", or the
specific tool name) and add a short parenthetical explanation plus a link to the
appropriate documentation or README for the agent (or to the development/agents
page) so readers know which agent to use and where to find usage/installation
instructions; keep the existing mention of the CLI but indicate it is being
phased out in favor of the named agent.

In `@tests/SegmentationPanel.spec.ts`:
- Around line 23-29: Replace direct locator count assertions on panel.rows with
the page-object API: call panel.getSegmentCount() and assert using
expect(count).toBe(expected). Specifically, change the two places where the test
uses await expect(panel.rows).toHaveCount(1) (and the similar assertion at lines
40-41) to use const count = await panel.getSegmentCount(); await
expect(count).toBe(1) (or expect(count).toBe(...)), while keeping the segment
text check via panel.nthSegment(0).locator; this preserves the standardized
page-object abstraction (panel.getSegmentCount, panel.nthSegment).

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.circleci/config.yml:
- Around line 434-441: The CS3D integration setup step in the CircleCI config is
currently enabled for every CYPRESS job and can be redirected by a committed
.cs3d-ref. Add explicit branch-based gating like the Playwright workflow around
the setup-cs3d.sh invocation, or make the job fail fast on protected branches
when .cs3d-ref exists, so master CI cannot consume a repo-pinned ref
accidentally.

In @.scripts/ci/setup-cs3d.sh:
- Around line 67-74: The reuse path in setup-cs3d.sh is too permissive: when
libs/@cornerstonejs already exists, it skips cloning without verifying that the
checkout matches the requested $REPO and $BRANCH. Update the existing-checkout
branch in the checkout logic to inspect the current repository’s origin and
resolved ref before building in place, using the same symbols around the git
clone condition, and hard-fail with a clear cleanup message if the local
checkout does not match the requested revision.